Skip to content

Conversation

ghost-not-in-the-shell
Copy link
Contributor

@ghost-not-in-the-shell ghost-not-in-the-shell commented May 18, 2022

Explain your changes:
This PR adds hard-coded checks for signature of timed account.

Explain how you tested your changes:
I added a new unit test for the created of timed account without signature.

Checklist:

  • Modified the current draft of release notes with details on what is completed or incomplete within this project
  • Document code purpose, how to use it
    • Mention expected invariants, implicit constraints
  • Tests were added for the new behavior
    • Document test purpose, significance of failures
    • Test names should reflect their purpose
  • All tests pass (CI will check this if you didn't)
  • Serialized types are in stable-versioned modules
  • Does this close issues? List them
  • Closes #0000

@ghost-not-in-the-shell ghost-not-in-the-shell added the ci-build-me Add this label to trigger a circle+buildkite build for this branch label May 18, 2022
@ghost-not-in-the-shell ghost-not-in-the-shell changed the title [WIP] Feature/timing update requires signature Feature/timing update requires signature May 20, 2022
let timing = Party.Update.timing party in
let local_state =
Local_state.add_check local_state
Creation_of_timed_account_without_signature
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you use the existing Update_not_permitted_timing_existing_account, and remove that old check for account_is_new?

Copy link
Member

@deepthiskumar deepthiskumar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Discussing cases where we want vesting schedules to be immutable

in
check_zkapp_failure Transaction_status.Failure.Overflow result ) )

let%test_unit "zkApp command, create timed account with wrong authorization"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you write a test for correct authorization and a test for the new case (fails when the account is already timed)?

let state_view =
Transaction_snark_tests.Util.genesis_state_view
in
let result =
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

call apply_zkapp_commands_at_slot instead (for all tests you added)? It applies both in snark and out of snark

@deepthiskumar deepthiskumar merged commit a700c1d into develop May 26, 2022
@deepthiskumar deepthiskumar deleted the feature/timing-update-requires-signature branch May 26, 2022 07:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci-build-me Add this label to trigger a circle+buildkite build for this branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants